Describe instrument protocols as data, and let SutterDevice speak through one - #118

Problem: the struct-format description carries the layout in one place and the
field names in another, so the two can drift; the read length is a third
thing, computed from the format; and a reply's terminator arrives as a pad
byte that is discarded rather than a value that can be checked. Meanwhile
struct.pack is positional, so nothing is named at the point of the call.

Solution: FrameRequest and FrameReply, describing the same frames as a ctypes
Structure. One declaration serves both directions -- bytes(frame) writes it,
from_buffer_copy() reads it back by name -- and sizeof() gives the read
length, so it cannot drift from the layout. Tests assert byte-for-byte
equality with the struct-format version on the MP-285 MOVE and GET_POSITION
frames, so the two can be compared on the same protocol rather than in the
abstract.

The variant brings a trap with it, which requirePackedLayout turns into an
error at description time: without _pack_ = 1, ctypes aligns each field to its
natural boundary and the 14-byte MOVE frame silently becomes 20, correct for a
C compiler and wrong for the stage. struct's "<" gets that right by default,
and the test pins the numbers.

Both variants plug into the same Exchange, so the choice is not load-bearing
yet.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Problem: running the prototype on Python 3.14 warns twice --

  DeprecationWarning: Due to '_pack_', the 'MoveFrame' Structure will use
  memory layout compatible with MSVC (Windows). If this is intended, set
  _layout_ to 'ms'. The implicit default is deprecated and slated to become an
  error in Python 3.19.

It does not appear on the 3.13 in the venv, so the tests looked clean while
the CI matrix, which runs up to 3.14, would have shown it.

Solution: state _layout_ = "ms" on both packed structures. With _pack_ = 1
there is no padding for the two layouts to disagree about, so this only names
what was already happening -- the test asserting byte-for-byte equality with
the struct format still passes on both interpreters. Python 3.13 and earlier
ignore the attribute, so it is safe across the supported range.

AlignedFrame is deliberately left alone: it declares no _pack_, because its
whole purpose is to be the aligned layout the description refuses.

Worth remembering when choosing between the two binary variants: ctypes
carries this deprecation, and struct does not.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The ctypes layout was worth trying and is not worth keeping. It brought two
real advantages -- one declaration serving both directions, and a named
terminator that a driver can assert instead of a discarded pad byte -- but it
charges for them twice in the same area. _pack_ = 1 is mandatory or the frame
silently gains alignment padding, which needed requirePackedLayout to catch;
and Python 3.14 then deprecates leaving the layout implicit alongside _pack_,
which needed _layout_ = "ms". Two pieces of ceremony to obtain the bytes that
"<clllc" produces by default.

So the description stays as two string notations, each the one Python already
provides for the job: a str.format template for text, a struct format for
binary. The file is byte-identical to 88d4e2d, before the variant landed.

The two advantages are not lost, only unclaimed: a struct format still
inverts through unpack, and BinaryReply can name the terminator as a field
rather than writing it off as padding, if that turns out to matter.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

On the way out a terminator is simply more literal text, so a separate
parameter bought nothing and cost clarity: TextRequest("pa?") silently wrote
b"pa?\r", and the Integra, which terminates nothing, had to opt out of the
default with terminator="". A description whose visible content differs from
what goes on the wire is the wrong kind of surprise.

Now every template carries its own line ending, and the differences between
instruments are legible side by side: "p {power:0.3f}\r" for the Cobolt,
"g r{register}\n" for the Intellidrive, "*GWL" for the Integra with nothing
at all.

TextReply keeps its terminator, because there it is not content. It is the
instruction for how far to read, the counterpart of BinaryReply.readLength,
and it cannot be written into a pattern that is only applied after the reply
has already been read.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

TextReply carried a terminator it never used: decode() only ever ran a regex
over whatever arrived. It was there to tell a caller how far to read, but the
port already knows that -- CommunicationPort.readString reads up to its own
terminator -- so the description was duplicating, and could contradict, a
setting that lives elsewhere. Its default of "\r\n" was already wrong for the
Intellidrive, which answers with "\r".

Now the same description reads a line however that line happened to arrive:
the test decodes b"0.123\r\n", b"0.123\n", b"0.123\r", b"0.123" and "0.123"
through one TextReply.

BinaryReply keeps readLength, and the asymmetry is the point rather than an
oversight: a fixed-size frame has no terminator to stop at, so the number of
bytes to ask for can come from nowhere but the description. It is the one
thing a caller cannot work out for itself.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Two changes to the prototype.

Require a byte-order prefix on every struct format. Without one, struct falls
back to native sizes and native alignment: "clllc" is 33 bytes on this machine
rather than the 14 the MP-285 expects, an "l" is whatever a C long happens to
be, and padding appears between the fields. pack("cl", b"M", 1) returns 16
bytes, seven of them padding. The frame is then right for the compiler and
wrong for the wire, and nothing downstream would notice.

That is the same failure the ctypes variant needed _pack_ = 1 to avoid, which
was half the reason for dropping it -- so it is worth saying plainly that
struct has the trap too, and that the difference is one character rather than
two class attributes. requireExplicitByteOrder now refuses the format at
description time and names both sizes.

Rename Exchange to Transaction. It is what the library already calls the
pairing of a write and its read: CommunicationPort.transactionLock guards
exactly that, keeping the two together against other threads.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The class describes what to send and what should come back. It carries
neither the data nor the transmission, so Command is the accurate word:
Exchange and Transaction both name the round trip, which is precisely the
part it does not do.

It takes the name of the class it would replace, which is right rather than
awkward -- the difference is not in what the thing is called but in what it
holds. Today's Command describes the protocol, builds the bytes, performs the
exchange, and then keeps the reply on itself, on a class attribute shared by
every instance of a driver. This one keeps a request and a reply description,
and hands the result to the caller.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

A driver's protocol is a table, and a table is data. CommandDictionary.
fromFile reads one device's commands and builds the same objects the tests
build by hand -- asserted directly: the JSON MOVE and the hand-written MOVE
produce the same bytes, and both equal pack("<clllc", b"M", ...). The layer
adds notation, not behaviour.

JSON rather than TOML or YAML because it is in the standard library of every
Python the package supports; tomllib arrives in 3.11 and the matrix still
runs 3.9, and YAML would be a dependency for a file nobody edits at runtime.

    "GET_POWER": {
      "request": {"template": "pa?\r"},
      "reply":   {"pattern": "(\\d+\\.\\d+)", "fields": {"power": "float"}}
    }

A request carries a template, for text, or a format with fields and
constants, for binary; a reply carries a pattern or a format. Which key is
present says which kind it is, so nothing has to declare a type twice.

The one thing a file cannot carry is a callable, so a text field names its
converter -- float, integer, text, boolean01, hexInteger -- and the name is
resolved against a table here. Keeping that list short is deliberate: a
protocol file describes a protocol, and anything wanting real code belongs in
the driver.

Descriptions are checked as they are read, since a typo in a file should not
become a puzzling failure on the wire: a command with no request, a request
that is neither text nor binary, and a converter that does not exist each
name the offending command, and the last names the converters that do exist.
The byte-order guard fires through this path too, so a file is not a way
around the checks the objects make. An unknown command name reports what the
device does have.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Five module-level functions and a dict existed only to build one class's
objects, so they belonged to it. converters is now a class attribute and
commandFrom, requestFrom, replyFrom, convertersFor and bytesFrom are
classmethods on CommandDictionary; the module namespace keeps only
requireExplicitByteOrder, which guards a format wherever one is written.

Reading order follows use: the entry points first -- fromFile, fromJSON,
fromDescription -- then the machinery they call, then the dict protocol.

Being classmethods rather than functions also makes them the extension point.
A device whose protocol needs something the description does not cover now
subclasses and overrides one of them instead of the module growing another
function, and because convertersFor reads cls.converters, a subclass adding a
converter of its own works with no other change:

    class MillenniaCommands(CommandDictionary):
        converters = dict(CommandDictionary.converters,
                          wattsFromMilliwatts=lambda text: float(text) / 1000)

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Fifteen methods had none: the two abstract hooks, every __init__, every
encode and decode, and the dict protocol on CommandDictionary. The project
asks for docstrings on all methods, and these are the ones a driver author
reads first.

They say what is returned and, where it matters, what is raised and why --
that TextRequest.encode raises MissingArgument naming the field rather than
letting a KeyError out of str.format, that decode raises ReplyDidNotMatch
quoting both sides, that Command.decode refuses when the command expects no
reply because decoding one means the caller read something it should not
have. BinaryReply.readLength says the length is taken from the format itself,
which is the property the tests pin.

Test methods are left alone: their names are already the sentences a
docstring would repeat.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Problem: the prototype could only say half of what a driver needs. It built a
request and read a reply, but not the mirror -- reading a request and writing a
reply -- which is exactly what a table-driven mock does. The Sutter protocol made
the gap concrete: SutterDevice's commands dict needed a second struct format to
write a position ('<lllc') next to the one that reads it ('<lllx'), and the two had
already drifted, one of them silently packing a null byte where the instrument
sends a carriage return. Nothing could say that an acknowledgement must be b"\r",
so sutterdevice.py checks it by hand three times. The keys of a description named
three notations inconsistently, and nothing told a reader how to call a command.

Solution: one Frame base requires encode and decode of every half, so a request
and a reply are each readable and writable and the driver and the mock share one
description. Command gains decodeRequest and encodeReply; CommandDictionary gains
recognize, which asks each command in turn whether the bytes are its request.

A binary frame states one struct format, because pack and unpack are exactly each
other's inverse; asking for a second is what let the old pair drift. Constants are
now stated once and serve both ways -- written on the way out, required on the way
in -- which is how a mock tells one command from another and how MOVE, HOME and
WORK finally refuse an acknowledgement that is not b"\r". Padding is gone from the
reply formats, since 'x' unpacks but does not pack.

Text cannot be derived either way: a template does not yield a regular expression
without guessing a pattern per format spec, and a regular expression yields no line
at all. Both are therefore required and neither is inferred, the same refusal to
guess that already makes the byte-order prefix mandatory. The keys and the
attributes now agree word for word: template and regex for text, struct for binary.

CommandDictionary.usage returns the commands, their arguments and what they answer,
with types read off the converters and the struct codes, constants left out because
nobody passes a header. print(dictionary) shows it.

The whole Sutter protocol is described, all four commands, and tested against the
bytes the driver produces today.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

… one

Problem: nine classes described two notations. Frame, Request, Reply, TextFrame,
TextRequest, TextReply, BinaryFrame, BinaryRequest and BinaryReply, of which six
were empty or nearly so, in a 2x2 grid that a third notation would have grown by
three. Request and Reply carried one attribute between them, mismatch, which is
not a property of a frame at all: whether bytes are a request or a reply depends
on the slot of the command they belong to, and that was already written there. The
same fact was stated twice, which is what this description exists to avoid.

The text pair had no common base either, so everything they shared had leaked out
to module scope: encodeTemplate, matchAndConvert, typeNameOf, structCodes. The
clearest symptom was matchAndConvert taking a mismatch parameter, which was only
ever self.mismatch -- an attribute passed as an argument because the code holding
it was not a method.

Solution: TextFrame, the symmetric counterpart of BinaryFrame, holds everything the
two text halves shared. encodeTemplate and matchAndConvert stop existing, because
they were encode and decode; typeNameOf and structCodes become methods of the frame
that uses them, and the struct type names a class attribute a subclass can extend,
the way CommandDictionary does with converters.

Request, Reply and the four role classes are then gone. A frame raises the plain
DidNotMatch; Command.decodeHalf turns that into RequestDidNotMatch or
ReplyDidNotMatch depending on which slot was being read, and names itself while it
is there -- so the message gained the one thing no frame could supply:

    before  ReplyDidNotMatch: expected '(\d+\.\d+)', got 'syntax error'
    after   ReplyDidNotMatch: GET_POWER: expected '(\d+\.\d+)', got 'syntax error'

Text now has one signature in both slots, (template, regex, fields), so the two
strings can no longer be given the wrong way round. Two tests hold the decision:
one that a frame reports only that it did not match while the command supplies
which half of which command, and one that puts a single frame in both slots of an
echo command, which the old hierarchy made impossible to write.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Problem: the docstrings explained why the design is what it is and almost never
what to pass or what comes back. Several signatures left the answer out too --
decode(self, data), typeNameOf(converter), decodeHalf(self, half, data, mismatch)
-- so nothing said whether data was bytes or a str, or that mismatch was an
exception class rather than an instance. Where the prose did try, it reached for
words that mean something else in Python: "one keyword per {name}" for what is
simply a named value. And the JSON example in CommandDictionary showed only a
request that takes no arguments, so nothing in the documentation revealed that a
text request can carry any, or what fields means on either side of a command.

Solution: every method of the prototype now carries Args, Returns and Raises, in
the Google style the recent core files already use (devicecontroller.py,
debugport.py, commands.py), keeping the prose that was there and adding the
sections underneath. An AST pass over the file checks that no method is missing
one and that no parameter or return is left unannotated.

The annotations use typing.Union rather than the 3.10 pipe, since pyproject sets
the floor at 3.9. **values stays object on purpose, and now says why: the type a
value must have belongs to the field it goes into, not to the method -- an int for
a struct "l", anything formattable for a text template -- so parameters answers
it, one name at a time, and a Union would be at once too narrow for text and too
wide for binary.

Along the way, and prompted by reading it back: requireExplicitByteOrder becomes a
classmethod of BinaryFrame with its prefixes as a class attribute, so a subclass
can override the pair instead of working around a module function; a frame is
called a frame rather than "this half", which now only means one of a command's
two sides; and the vaguest docstrings were rewritten to show what they describe
("power",) for "p {power:0.3f}\r" rather than "the names this line carries".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Problem: struct allows whitespace between codes and ignores it, so "<l l" is a
valid two-long format. structCodes took the space for a code of its own and
returned ('l', ' ', 'l'), one entry too many. Since the codes are zipped against
the field names to give each one a type, every name after the space was reported
with its neighbour's type, and the last one with none.

Nothing on the wire was affected: structCodes serves parameters, and parameters
serves nothing but usage(). The damage was a usage line that lied about types --
which is worth fixing precisely because a usage line is what someone reads instead
of the protocol file.

Solution: skip whitespace, as struct does. Its docstring now also says that,
unlike typeNameOf next door, this is not a guess but a reading of a closed and
documented set of codes, so it is expected to be exact. The test covers both
levels: the codes themselves, and that a frame written "<c lll c" still announces
x, y and z as int32.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

…h it

Problem: the protocol description lived inside its own test file, so nothing
could use it. SutterDevice still spoke through the commands dict of
communication/commands.py, whose DataCommand describes the protocol, builds the
bytes, performs the I/O, and then keeps the reply on itself -- on a class
attribute shared by every instance of the driver. It needed a second struct
format to write a position ('<lllc') beside the one that reads it ('<lllx'), and
those two had drifted: one of them silently packed a null byte where the
instrument sends a carriage return. Nothing could say that an acknowledgement must
be b"\r", so the driver checked it by hand three times. And the debug port was a
nested class with a process_command full of branches, holding a second table of
prefixes that had to be kept in step with the driver by hand -- badly, as it
turned out: it accepted a lowercase 'm' and a request with no terminator, neither
of which an MP-285 would, and two tests were passing against bytes that could
never have worked on a bench.

Solution: communication/protocol.py. A Frame turns named values into bytes and
bytes back into named values, owning no port and storing nothing. A binary frame
states one struct format, since pack and unpack are exactly each other's inverse;
a text frame states both its notations, since neither can be derived from the
other without guessing. Constants are stated once and serve both directions,
written on the way out and required on the way in, which is what closes both of
the Sutter's old gaps at once.

PhysicalDevice gains protocol (None by default) and performTransaction, which
performs one command under the port's transactionLock, reading by readLength for
a frame and up to the terminator for a line, through the primitives of
CommunicationPort and nothing else. Unlike sendCommand it does not require the
device to be Ready, which is exactly why the old SutterDevice had to reach around
sendCommand during initialization.

ProtocolDebugPort then needs no code of its own at all: whatever a request carries
is remembered by name, whatever a reply carries is answered from that memory, and
recognize() picks the command out of the same description the driver sends with.
One clause had to be invented for it -- "sets", for a state change the bytes
cannot express, since HOME carries nothing and still moves the stage. It is the
only part of a description that talks about the instrument rather than the wire,
and a driver never reads it.

CommandDictionary.validate() refuses a description that is wrong about itself: two
notations describing different lines, a struct packing a different number of
values than it names, a command whose request another command answers to first.
It makes specimen values from the declared types, so a protocol file is checked
without a line of test data. SutterDevice's own description is validated on every
run of its test.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Problem: sendCommand looked a command up in self.commands, sent it through
self.port, and handed the Command object back so a caller could read the reply
off it -- .reply, .matchGroups, .exceptions. That is the pattern the protocol
description exists to replace: the reply lands on an object shared by every
instance of the driver, and two callers of one command overwrite each other. It
also refused to run unless the device was Ready, which is why SutterDevice used
to reach around it during initialization to check the stage was answering.

Nothing in the library called it. The two tests that did -- testEchoCommands, in
its hardware and debug flavours -- iterated EchoDevice.commands and sent each one.

Solution: delete it. Those two tests now call the Command's own send() with the
device's port, which is all sendCommand ever did once the state check is gone.
performTransaction is its successor for a driver that sets a protocol.

self.commands stays for now: EchoDevice, CoboltDevice and IntellidriveDevice use
theirs to build a TableDrivenDebugPort, and IntegraDevice is the last one that
still sends with it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>